-
Notifications
You must be signed in to change notification settings - Fork 159
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat: Add emitMulti with Spliterator support #1776
Conversation
@pjfanning Would you like to take a look at this, thank. |
Can you give some background on who needs this? Is it intended that anyone can use this or do you just want it for #1775? |
For now, only in #1775 , but it's generally |
the code is protected - so other users won't easily be able to access it |
stream-tests/src/test/scala/org/apache/pekko/stream/impl/GraphStageLogicSpec.scala
Outdated
Show resolved
Hide resolved
@@ -1118,6 +1139,19 @@ abstract class GraphStageLogic private[stream] (val inCount: Int, val outCount: | |||
} | |||
} | |||
|
|||
private final class EmittingSpliterator[T](_out: Outlet[T], elems: Spliterator[T], _previous: OutHandler, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- the advantage of a Spliterator over an iterator is that you can use trySplit() to partition the workload so you can get multiple threads to work on the data
- if you just use tryAdvance, you might as well be just using the standard iterator]
- so what do we gain here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
you can avoid the hasNext
and next
call when you already have a Spliterator
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure this will make a big diff to performance. I'll approve this but without jmh bechmarks, I am very sceptical about whether this extra code is worth the extra maintenance overhead.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm
Motivation:
Add Spliterator support for emitMulti, reducing overhead when working with java stream.
Result:
less overhead
I would like rebase #1775 on this.